fix: bind hash-path attribution memos to challenge IDs#190
fix: bind hash-path attribution memos to challenge IDs#190brendanjryan wants to merge 4 commits intomainfrom
Conversation
brendanjryan
force-pushed
the
fix/challenge-bound-hash-memo
branch
from
81c330f to
7319ccc
Compare
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81c330f9f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if charge.memo().is_none() { | ||
| assert_challenge_bound_memo(&matched_logs, challenge_id, realm)?; | ||
| } |
There was a problem hiding this comment.
This new guard rejects every hash-path payment without an explicitly requested memo unless at least one matched log contains a challenge-bound MPP memo. That breaks mixed-version deployments: older clients generated random attribution memos (not challenge.id-bound), so otherwise valid receipts now fail with memo is not bound to this challenge as soon as servers upgrade. Consider a compatibility path (e.g., temporary legacy memo acceptance or version-gated enforcement) so rolling upgrades do not hard-fail existing clients.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is intentional for hash based flows, which are not a common path
|
|
||
| /// Encodes an MPP attribution memo as a 32-byte array. | ||
| pub fn encode(server_id: &str, client_id: Option<&str>) -> [u8; 32] { | ||
| pub fn encode(challenge_id: &str, server_id: &str, client_id: Option<&str>) -> [u8; 32] { |
There was a problem hiding this comment.
Changing mpp::tempo::attribution::encode to require challenge_id is a source-compatible break for downstream users calling this public API, and encode_hex was changed similarly. Shipping this as a patch means consumers can pick up the update and hit compile failures unexpectedly; a backward-compatible wrapper/deprecation path (or a compatibility-level bump) is needed to prevent that breakage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is ok given maturity of this package
👁️ Cyclops Security Review🧭 Auditing · mode=
⚙️ Controls
|
2 participants
port of https://github.com/wevm/mppx